Repository navigation
Create wmin/wmax on the device of the given weights - #837
Merged
Merged
Conversation
A connection built from GPU weights failed in its constructor's clamp
("Expected all tensors to be on the same device"): AbstractConnection
always created the bounds on the CPU. They now follow w's device; without
w they stay on the CPU as before. Single-value and per-weight bounds both
work. No change in results.
Test: test_connection_from_gpu_weights_clamps_and_learns_on_gpu fails on
master on a CUDA machine and passes with the fix (CI has no GPU).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes and why
A connection built from weights already on a GPU (
Connection(..., w=w_cuda, wmin=0.0, wmax=1.0))failed in its constructor with
Expected all tensors to be on the same device, but got min is on cpu.AbstractConnection.__init__always createdwminandwmaxon the CPU, and the subclassconstructors clamp the given
wagainst them. The bounds are now created on the device of thegiven
w. Withoutwthe weights start on the CPU, and so do the bounds, as before.Reported on Intel XPU (Aurora); the same error happens on CUDA.
Casting the bounds to
floatavoids the error only for single-value bounds; per-weight bounds(a tensor the size of
w, which the docstring allows) cannot be cast. This fix covers both.Checklist
poetry run pytestlocally and it passed. (644 passed, on a machine with CUDA)masterand passes with my change(checked by the
bug-testcheck):test/network/test_connections.py::test_connection_from_gpu_weights_clamps_and_learns_on_gpuIt needs CUDA, so on CI (no GPU) it is skipped and
bug-testcannot see it. On a CUDAmachine both parameter cases fail on
masterwith the device error and pass with the fix.The new GPU test carries
skipif(not torch.cuda.is_available()), liketest_windows_and_maxpool_state_stay_on_gpuin the same file.- [x] No. Only the device of the bounds changes, and only when
wis given.Unreleasedsection ofCHANGELOG.md.AI assistance
Tools used and for which parts: Claude Code reproduced the bug, wrote the fix, the tests and the
changelog entry.
Not in this pull request
Conv1dConnection,Conv2dConnection,Conv3dConnectionandMeanFieldConnectionclamp a givenwonly when a bound is infinite ((self.wmin == -inf).any() or (self.wmax == inf).any()), thereverse of
Connection(!=). With finite bounds a givenwis never clamped. Fixing it changesresults for anyone passing out-of-range weights, so it belongs in its own pull request.